Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new Poco::SSH wrapper library (client/server utilities built on libssh), plus a CppUnit testsuite and a sample CLI client.
Changes:
- Registers the new
SSHcomponent and adds build/packaging files (CMake + Makefiles). - Introduces
Poco::SSHcore APIs:SSHServer,SSHSession,SSHClient,SSHHostKeyManager,SSHChannelStream, and exceptions. - Adds a testsuite (
SSH-testrunner) and apocosshsample client.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| components | Registers SSH as a top-level component. |
| SSH/CMakeLists.txt | Builds/install/packages the new SSH library and conditionally adds tests/samples. |
| SSH/Makefile | Adds make-based build for PocoSSH and links against libssh. |
| SSH/cmake/PocoSSHConfig.cmake | CMake package config for downstream find_package(PocoSSH). |
| SSH/dependencies | Declares dependency on Foundation. |
| SSH/include/Poco/SSH/SSH.h | Defines export macros and auto-link behavior for the SSH library. |
| SSH/include/Poco/SSH/SSHChannelStream.h | Declares stream wrappers and sshReadLine() helper. |
| SSH/include/Poco/SSH/SSHClient.h | Declares SSH client wrapper API. |
| SSH/include/Poco/SSH/SSHException.h | Declares SSH-specific exception types. |
| SSH/include/Poco/SSH/SSHHostKeyManager.h | Declares host key creation/loading utility. |
| SSH/include/Poco/SSH/SSHServer.h | Declares SSH server wrapper and session tracking. |
| SSH/include/Poco/SSH/SSHServerConfig.h | Declares server configuration struct and auth callback. |
| SSH/include/Poco/SSH/SSHSession.h | Declares server-side session handler base class. |
| SSH/src/SSHChannelStream.cpp | Implements ostream/channel bridge and line reader helper. |
| SSH/src/SSHClient.cpp | Implements the client wrapper (connect/auth/channel). |
| SSH/src/SSHException.cpp | Implements SSH exception classes. |
| SSH/src/SSHHostKeyManager.cpp | Implements Ed25519 host key generation/validation. |
| SSH/src/SSHServer.cpp | Implements server listen/accept loop, shutdown, and session tracking. |
| SSH/src/SSHSession.cpp | Implements key exchange, authentication, channel negotiation, and cleanup. |
| SSH/samples/CMakeLists.txt | Adds SSH sample(s) to the build. |
| SSH/samples/pocossh/CMakeLists.txt | Builds the pocossh sample client. |
| SSH/samples/pocossh/Makefile | Make-based build for pocossh sample. |
| SSH/samples/pocossh/src/pocossh.cpp | Adds an interactive SSH CLI client sample. |
| SSH/testsuite/CMakeLists.txt | Adds CTest integration for SSH tests. |
| SSH/testsuite/Makefile | Make-based build for SSH testsuite runner. |
| SSH/testsuite/src/Driver.cpp | Adds console test runner main() for SSH tests. |
| SSH/testsuite/src/SSHTest.cpp | Adds integration-style tests for server/client/auth/channel behavior. |
| SSH/testsuite/src/SSHTest.h | Declares SSH test case class. |
| SSH/testsuite/src/SSHTestSuite.cpp | Defines SSH test suite composition. |
| SSH/testsuite/src/SSHTestSuite.h | Declares SSH test suite entry point. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closed
1 task
matejk
force-pushed
the
5314-ssh-wrapper-lib
branch
from
9218b70 to
2398afb
Compare
Use pkg-config on Darwin to locate libssh headers/libs, replace deprecated ssh_pki_generate with ssh_pki_generate_key (libssh >= 0.11), and fix accept loop hang on macOS by using poll() instead of relying on shutdown() to unblock accept()
ssh_pki_generate_key was introduced in libssh 0.12.0, not 0.11.0; correct the version guard so builds against 0.11.x (vcpkg baseline on windows-2025) fall back to ssh_pki_generate. On Windows, shutdown() alone does not reliably unblock libssh's internal poll, so server shutdown with an active session hangs. Follow shutdown() with closesocket() to force the session's blocking read to error out and the session thread to exit.
matejk
force-pushed
the
5314-ssh-wrapper-lib
branch
from
2398afb to
20f6961
Compare
CI (Windows, windows-2025-msvc-cmake only): - Single OpenSSL source via vcpkg (remove the second FireDaemon install). - Pre-install scrub of stale runner-image OpenSSL/OpenSSH and cached vcpkg ports. Fixed-path assertions on DLLs/libs/headers plus legacy.dll. Post-install Get-Command check that libcrypto, libssl, ssh, and legacy DLLs resolve under the vcpkg bin dir. - Post-job vcpkg remove in if: always() step. - Pass --vcpkg-root=$VCPKG_INSTALLATION_ROOT to silence the VCPKG_ROOT mismatch warning from the VS-bundled vcpkg. - vcpkg binary cache via actions/cache@v5 on %LOCALAPPDATA%/vcpkg/archives. SSH library: - Switch from hand-written cmake/FindLibSSH.cmake to upstream find_package(libssh CONFIG); link against imported target ssh. Drops 73-line module; works across apt libssh-dev (Debian 12+, Ubuntu), brew libssh, and vcpkg libssh. - Pin classical KEX (curve25519/ecdh/dh-group16/14) in testsuite to work around libssh 0.12.0 ML-KEM hybrid KEX bug on MSVC x64 (upstream fix landed post-0.12.0 tag and is not yet in vcpkg's libssh port). Code review (clang-tidy-21 + manual): - acceptLoop: retry on EINTR instead of breaking. - authenticate(): bound total messages (10x maxAuthAttempts). - disconnectAllSessions: SHUT_RDWR/SD_BOTH instead of magic 2. - SSHHostKeyManager::generateKey: UmaskGuard RAII. - SSHClient::authenticatePublicKey: initialized rc. - Sample pocossh: ssize_t for read/write, zero-init pollfd/termios, explicit stub on Windows where the interactive loop is absent. - '= default' for trivial destructors; Driver uses reserve + emplace_back. - ASCII-only comments; #if defined / #if !defined throughout SSH/.
matejk
force-pushed
the
5314-ssh-wrapper-lib
branch
from
8ce887f to
493762b
Compare
Extend the actions/cache step to cover C:\vcpkg\downloads alongside the binary archives. The downloads dir holds source tarballs (openssl, libssh, ...) and tool installs (cmake, ninja, perl, powershell-core) that vcpkg re-downloads on every ABI-miss rebuild; persisting them avoids the ~30-60s cmake/powershell/perl zip download on every run where the binary cache misses. Key prefix renamed vcpkg-archives- -> vcpkg- since it now covers more than just archives. Old cache entries will be orphaned and LRU-evicted.
matejk
approved these changes
Apr 21, 2026
Contributor
matejk
left a comment
matejk
left a comment
There was a problem hiding this comment.
Updated CI and removed custom cmake files and rather used provided ones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.